Update taffy to 0.9 and fix Grid errors #21240 by rossleonardy · Pull Request #21672 · bevyengine/bevy

rossleonardy · 2025-10-28T00:20:03Z

Objective

Css grid cell spanning (GridPlacement) produces incorrect layout #21240
Update taffy to 0.9.1 and grid placement with start and ends works again.

Solution

I had to make some rather un-ergonomic changes I would like advice on, so I've left comments below.

Testing

Pending...

Showcase

Fixes issue with grid start and ends...

crates/bevy_ui/Cargo.toml

rossleonardy · 2025-10-28T00:23:26Z

crates/bevy_ui/src/layout/convert.rs

            .collect::<Vec<_>>(),
        grid_row: node.grid_row.into(),
        grid_column: node.grid_column.into(),
+        ..Default::default()


The fields added are dummy, item_is_replaced, grid_template_areas, grid_template_column_names, grid_template_row_names. Not sure if this should be using their default or we should add these fields and have our own default.

dummy is just phantomdata. grid_template_areas, grid_template_column_names, and grid_template_row_names should all be left at their default values for now.

I'm confused about is_replaced. The CoreStyle trait seems to map it to is_compressible_replaced which isn't quite same thing as a replaced element, like a form field isn't compressible normally?

rossleonardy · 2025-10-28T00:25:21Z

crates/bevy_ui/src/layout/convert.rs

+                Val::Px(val)
+                    .into_length_percentage(context)
+                    .into_raw()
+                    .value(),


These rather verbose conversions are due to the fact that we do some math in into_length_percentage and into_length_percentage_auto, I would like to refactor the Val type story but this keeps the behavior the same

rossleonardy · 2025-10-28T00:26:14Z

crates/bevy_ui/src/layout/convert.rs

+    ) -> taffy::style::GridTemplateComponent<S>
+    where
+        S: CheapCloneStr,
+    {


This was an add things until it compiles moment, if anyone can explain what the generics mean here I would appreciate it

Taffy is now generic over a String-like type (which allows for named grid lines and named grid areas to be specified). Bevy should feel free to pick a concrete type such as Arc<str>, string_cache::Atom (or even String, but the idea is that such strings are cheaply cloneable).

If you're not actually using the named lines/areas feature (and thus such types will never actually be constructed) then you might consider Box<String> which would keep the size impact minimal.

github-actions · 2025-10-28T00:26:43Z

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21672

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

rossleonardy · 2025-10-28T00:29:51Z

crates/bevy_ui/src/layout/ui_surface.rs

    }
 }

+pub(crate) struct BevyTaffyTree<T>(TaffyTree<T>);


This is sort of a patch for an issue with Taffy:
ccbrown/iocraft#119 (comment)
Eventually it will be fixed in
DioxusLabs/taffy#823

github-actions · 2025-10-28T01:17:24Z

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21672

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

github-actions · 2025-10-28T01:24:22Z

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21672

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

github-actions · 2025-10-28T02:00:06Z

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21672

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

github-actions · 2025-10-28T14:15:54Z

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21672

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

ickshonpe · 2025-10-29T09:45:43Z

I'll try and make time to take a look at this later today, chase me up about it if I haven't.

crates/bevy_ui/src/widget/image.rs

nicoburns · 2025-10-29T19:34:22Z

crates/bevy_ui/src/layout/convert.rs

+                Val::VMin(val)
+                    .into_length_percentage(context)
+                    .into_raw()
+                    .value(),


I believe Taffy has an Into implementation for this conversion

ickshonpe

Looks good, performance is normal, examples all seem to be working fine. Don't see any major problems, just got a couple of notes.

ickshonpe · 2025-10-30T00:33:38Z

crates/bevy_ui/src/layout/convert.rs

+            MinTrackSizingFunction::Px(val) => taffy::style::MinTrackSizingFunction::length(
+                Val::Px(val)
+                    .into_length_percentage(context)
+                    .into_raw()
+                    .value(),


I think just:

Suggested change

MinTrackSizingFunction::Px(val) => taffy::style::MinTrackSizingFunction::length(

Val::Px(val)

.into_length_percentage(context)

.into_raw()

.value(),

MinTrackSizingFunction::Px(val) => Val::Px(val).into_length_percentage(context).into(),

should work, and so on for the rest.

Changed this, but left the two where its actually necessary

ickshonpe · 2025-10-30T00:36:53Z

crates/bevy_ui/src/layout/convert.rs

+            MaxTrackSizingFunction::Px(val) => taffy::style::MaxTrackSizingFunction::length(
+                Val::Px(val)
+                    .into_length_percentage(context)
+                    .into_raw()
+                    .value(),
            ),


Same here:

Suggested change

MaxTrackSizingFunction::Px(val) => taffy::style::MaxTrackSizingFunction::length(

Val::Px(val)

.into_length_percentage(context)

.into_raw()

.value(),

),

MaxTrackSizingFunction::Px(val) => Val::Px(val).into_length_percentage(context).into(),

ickshonpe · 2025-10-30T00:41:32Z

crates/bevy_ui/src/layout/ui_surface.rs

    }
 }

+pub(crate) struct BevyTaffyTree<T>(TaffyTree<T>);


The name's a bit too long and ugly, maybe just:

Suggested change

pub(crate) struct BevyTaffyTree<T>(TaffyTree<T>);

pub(crate) struct UiTree<T>(TaffyTree<T>);

would be better, or something like that.

ickshonpe · 2025-10-30T00:51:12Z

crates/bevy_ui/src/layout/ui_surface.rs

+#[expect(unsafe_code, reason = "TaffyTree is safe as long as calc is not used")]
+/// SAFETY: Taffy Tree becomes thread unsafe when you use the calc feature, which we do not implement
+unsafe impl Send for BevyTaffyTree<NodeMeasure> {}
+
+#[expect(unsafe_code, reason = "TaffyTree is safe as long as calc is not used")]
+/// SAFETY: Taffy Tree becomes thread unsafe when you use the calc feature, which we do not implement
+unsafe impl Sync for BevyTaffyTree<NodeMeasure> {}


Couldn't we just implement the traits for UiSurface itself?

Probably better to do it like it's already being done here I think. Lest we accidently add some non-Send field to another part of UiSurface. Implementing Deref and DerefMut for BevyTaffyTree would probably make this more ergonomic.

Yeah it's not a user facing API anyway, so the extra friction doesn't really matter. We could hide it completely even, and add an accessor to UiSurface that returns a reference to the inner TaffyTree value.

Since it will be removed after DioxusLabs/taffy#855 or similar work goes through, I think its nicer if it's a little separated like this.

Renamed to UiTree

…21672) # Objective - bevyengine#21240 - Update taffy to 0.9.1 and grid placement with start and ends works again. ## Solution I had to make some rather un-ergonomic changes I would like advice on, so I've left comments below. ## Testing - Pending... --- ## Showcase <img width="912" height="740" alt="Screenshot 2025-10-27 at 8 18 50 PM" src="https://github.com/user-attachments/assets/50ec2de2-288f-4f8a-9e20-216bd9d1474d" /> Fixes issue with grid start and ends...

rossleonardy added 2 commits October 27, 2025 20:11

Update to taffy 0.9

3b0e81b

Fix call here

6bc8a5e

rossleonardy changed the title ~~Update taffy to 9.0 and fix Grid errors #21240~~ Update taffy to 0.9 and fix Grid errors #21240 Oct 28, 2025

rossleonardy commented Oct 28, 2025

View reviewed changes

crates/bevy_ui/Cargo.toml Outdated Show resolved Hide resolved

rossleonardy commented Oct 28, 2025

View reviewed changes

rossleonardy added 2 commits October 27, 2025 20:50

Fix ci suggestions

8fe8b0c

fix cargo toml

409e97b

JMS55 requested a review from ickshonpe October 28, 2025 01:15

min track sizing function fix

0d7a63b

Fix safety messages

92e1713

Fix maxtracksizingfunction

d5cd7e3

alice-i-cecile requested review from nicoburns and viridia October 29, 2025 18:47

alice-i-cecile added C-Dependencies A change to the crates that Bevy depends on A-UI Graphical user interfaces, styles, layouts, and widgets X-Uncontroversial This work is generally agreed upon S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 29, 2025

alice-i-cecile added this to the 0.18 milestone Oct 29, 2025

alice-i-cecile added X-Contentious There are nontrivial implications that should be thought through and removed X-Uncontroversial This work is generally agreed upon labels Oct 29, 2025

nicoburns reviewed Oct 29, 2025

View reviewed changes

crates/bevy_ui/src/widget/image.rs Show resolved Hide resolved

nicoburns reviewed Oct 29, 2025

View reviewed changes

ickshonpe reviewed Oct 30, 2025

View reviewed changes

This was referenced Oct 30, 2025

Update taffy requirement from 0.7 to 0.8 #20275

Closed

Update taffy requirement from 0.7 to 0.9 #20509

Closed

rossleonardy added 4 commits October 31, 2025 20:48

Fix cargo features for bevy ui

3305bf2

More into()

083d333

Rename bevytaffytree

bcf397b

Fix generics

7decca3

ickshonpe approved these changes Nov 3, 2025

View reviewed changes

nicoburns approved these changes Nov 3, 2025

View reviewed changes

rossleonardy requested a review from alice-i-cecile November 3, 2025 16:54

alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 3, 2025

alice-i-cecile added this pull request to the merge queue Nov 3, 2025

PPakalns mentioned this pull request Nov 3, 2025

Support taffy 0.8 PPakalns/egui_taffy#18

Open

2 tasks

Merged via the queue into bevyengine:main with commit f45b216 Nov 3, 2025
42 checks passed

	pub(crate) struct BevyTaffyTree<T>(TaffyTree<T>);
	pub(crate) struct UiTree<T>(TaffyTree<T>);

Uh oh!

Conversation

rossleonardy commented Oct 28, 2025

Objective

Solution

Testing

Showcase

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 28, 2025

Uh oh!

github-actions bot commented Oct 28, 2025

Uh oh!

github-actions bot commented Oct 28, 2025

Uh oh!

github-actions bot commented Oct 28, 2025

Uh oh!

ickshonpe commented Oct 29, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ickshonpe left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicoburns Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ickshonpe Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ickshonpe left a comment •

edited

Loading

nicoburns Oct 30, 2025 •

edited

Loading

ickshonpe Oct 30, 2025 •

edited

Loading